Conversation
| @@ -0,0 +1,5 @@ | |||
| import * as Sentry from '@sentry/electron/renderer'; | |||
|
|
|||
| export function setSentryWpcomUserIdRenderer( id: number | undefined ) { | |||
There was a problem hiding this comment.
I intentionally added postfix as ...Renderer and ...Main to simplify code reading, since sometimes it feels confusing and not clear - is it error from main or from renderer, so easy to make a mistake.
| import * as Sentry from '@sentry/electron/renderer'; | ||
|
|
||
| export function setSentryWpcomUserIdRenderer( id: number | undefined ) { | ||
| Sentry.setTag( 'wpcom.user.id', id ); |
There was a problem hiding this comment.
wpcom.user.id - I see that Sentry uses dot as a delimiter for tags, that's why I also went this way.
📊 Performance Test ResultsComparing c539154 vs trunk app-size
site-editor
site-startup
Results are median values from multiple test runs. Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff) |
|
We got an approval to proceed with this - p4H3ND-22K-p2 |
sejas
left a comment
There was a problem hiding this comment.
The code looks good to me, and I can confirm that Sentry captures the WP.com user ID for errors on the main thread. For the renderer, I only saw the first trigger and not the subsequent ones. See the number of reported issues in the screenshot.
I tested this PR following your testing instructions and applying your diff.
I started with an authenticated user, then I logged out and the main process was still showing my existing userId. When I authenticated with another user I saw the correct userId. So, from my point of view is good enough.
My only concern is about the renderer not sending the following errors. Am I doing anything wrong? Maybe is an existing issue/feature of Sentry? 🤔
@sejas Thanks, yeah, I also noticed it and mentioned in the description And decided to look deeper and indeed - Sentry ships with a built-in deduplication flag, that is enabled by default. It suppresses consecutive events that have the same exception message and stack trace. I think it makes sense, to prevent flooding Sentry with duplicates. Anyway, it's existing, default Sentry SDK behavior and we already have it in Studio, so changes in this PR don't change it. |


Related issues
How AI was used in this PR
AI generated code, we had many iterations, and ultimatelly partially it was written by me manually to polish it.
Proposed Changes
I had a case when I knew the wpcom userId, but I didn't know the sentryUserId. I asked the user about sharing it with me, but they messed it up by removing appdatav1.json and not reloading Studio.
So if we add wpcom userId to Sentry then:
Testing Instructions
npm start